Add detection for in-project Poetry environments and support for poetry.toml #283
Conversation
karthiknadig
added
the
bug
There was a problem hiding this comment.
Pull request overview
This pull request adds detection for in-project Poetry environments (.venv directories) by implementing a fallback identification mechanism in the try_from() method. This addresses issue #282 where Poetry environments created with virtualenvs.in-project = true were not being detected when workspace directories weren't configured or when the pyproject.toml wasn't found during the initial find() phase.
Changes:
- Renamed
is_poetry_environmenttois_poetry_cache_environmentto clarify it detects cache-based Poetry environments - Added
is_in_project_poetry_environmentfunction that detects.venvdirectories with Poetry configuration - Updated tests to cover both cache-based and in-project Poetry environment detection scenarios
- Added tempfile dev dependency for filesystem-based testing
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/pet-poetry/src/lib.rs | Renamed cache detection function and added new is_in_project_poetry_environment function with fallback detection logic in try_from() |
| crates/pet-poetry/tests/path_identification_test.rs | Renamed test functions for cache detection and added comprehensive tests for in-project Poetry environments |
| crates/pet-poetry/Cargo.toml | Added tempfile dev dependency for testing |
| Cargo.lock | Updated with tempfile dependency |
Comments suppressed due to low confidence (1)
crates/pet-poetry/tests/path_identification_test.rs:291
- Consider adding test coverage for false positive scenarios where Poetry-related strings appear in comments or documentation within pyproject.toml. For example, a file containing:
# This project does not use Poetry. See poetry-core documentation at...
# [tool.poetry] section would go here if we used Poetry
This would help ensure the detection logic doesn't incorrectly identify non-Poetry environments.
#[test]
fn test_in_project_non_poetry_env_rejected() {
let temp_dir = tempfile::tempdir().unwrap();
let project_dir = temp_dir.path();
let venv_dir = project_dir.join(".venv");
// Create .venv directory
fs::create_dir(&venv_dir).unwrap();
// Create pyproject.toml without Poetry configuration
let pyproject_content = r#"
[project]
name = "my-project"
version = "0.1.0"
[build-system]
requires = ["setuptools>=45"]
build-backend = "setuptools.build_meta"
"#;
fs::write(project_dir.join("pyproject.toml"), pyproject_content).unwrap();
// Test that the .venv is NOT recognized as a Poetry environment
assert!(!test_in_project_poetry_env(&venv_dir));
}
#[test]
fn test_in_project_env_no_poetry_config_rejected() {
let temp_dir = tempfile::tempdir().unwrap();
let project_dir = temp_dir.path();
let venv_dir = project_dir.join(".venv");
// Create .venv directory without any Poetry configuration files
fs::create_dir(&venv_dir).unwrap();
// Test that the .venv is NOT recognized as a Poetry environment
assert!(!test_in_project_poetry_env(&venv_dir));
}
#[test]
fn test_in_project_poetry_env_with_poetry_toml() {
let temp_dir = tempfile::tempdir().unwrap();
let project_dir = temp_dir.path();
let venv_dir = project_dir.join(".venv");
// Create .venv directory
fs::create_dir(&venv_dir).unwrap();
// Create poetry.toml with in-project setting (no pyproject.toml with Poetry config)
let poetry_toml_content = r#"
[virtualenvs]
in-project = true
"#;
fs::write(project_dir.join("poetry.toml"), poetry_toml_content).unwrap();
// Create minimal pyproject.toml without Poetry-specific config
let pyproject_content = r#"
[project]
name = "my-project"
version = "0.1.0"
[build-system]
requires = ["setuptools>=45"]
build-backend = "setuptools.build_meta"
"#;
fs::write(project_dir.join("pyproject.toml"), pyproject_content).unwrap();
// Test that the .venv is recognized as a Poetry environment due to poetry.toml
assert!(test_in_project_poetry_env(&venv_dir));
}
#[test]
fn test_non_venv_directory_rejected() {
let temp_dir = tempfile::tempdir().unwrap();
let project_dir = temp_dir.path();
let custom_venv = project_dir.join("myenv");
// Create custom env directory (not named .venv)
fs::create_dir(&custom_venv).unwrap();
// Create pyproject.toml with Poetry configuration
let pyproject_content = r#"
[tool.poetry]
name = "my-project"
version = "0.1.0"
"#;
fs::write(project_dir.join("pyproject.toml"), pyproject_content).unwrap();
// Test that non-.venv directories are NOT recognized
assert!(!test_in_project_poetry_env(&custom_venv));
}
| if let Ok(contents) = std::fs::read_to_string(&pyproject_toml) { | ||
| // Look for [tool.poetry] or poetry as build backend | ||
| if contents.contains("[tool.poetry]") | ||
| || contents.contains("poetry.core.masonry.api") | ||
| || contents.contains("poetry-core") | ||
| { |
There was a problem hiding this comment.
The string matching approach using contains() is too broad and could result in false positives. For example, this would incorrectly identify a Poetry environment if "poetry-core" appears in a comment, URL, or documentation within the pyproject.toml file.
Consider using proper TOML parsing (similar to how it's done in pyproject_toml.rs and config.rs) to check the actual structure:
- Parse the TOML content with
toml::from_str::<toml::Value>() - Check for
tool.poetrysection existence - Check for
build-system.build-backendspecifically containing "poetry.core.masonry.api" - Check for
build-system.requiresarray containing items that start with "poetry-core"
This would provide more accurate detection and avoid false positives from arbitrary text content.
| if let Ok(contents) = std::fs::read_to_string(&pyproject_toml) { | ||
| if contents.contains("[tool.poetry]") | ||
| || contents.contains("poetry.core.masonry.api") | ||
| || contents.contains("poetry-core") | ||
| { | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
The test helper function duplicates the same overly broad string matching approach from the implementation. The tests should mirror the proper TOML parsing approach that would be used in the production code to ensure they're testing the correct behavior.
If the implementation is updated to use proper TOML parsing (checking tool.poetry section, build-system.build-backend, and build-system.requires), this test helper should be updated to match that logic.
This issue also appears in the following locations of the same file:
- line 202
Implement detection for in-project Poetry environments by checking for the presence of poetry.toml and pyproject.toml files. Update tests to ensure proper functionality.
fixes #282